Skip to content

feat: update mosquitto strings#3343

Open
Its-Just-Nans wants to merge 4 commits intoeclipse-mosquitto:developfrom
Its-Just-Nans:update-mosquitto-strings
Open

feat: update mosquitto strings#3343
Its-Just-Nans wants to merge 4 commits intoeclipse-mosquitto:developfrom
Its-Just-Nans:update-mosquitto-strings

Conversation

@Its-Just-Nans
Copy link
Copy Markdown
Contributor

@Its-Just-Nans Its-Just-Nans commented Aug 16, 2025

  • Have you signed the Eclipse Contributor Agreement, using the same email address as you used in your commits?
  • Do each of your commits have a "Signed-off-by" line, with the correct email address? Use "git commit -s" to generate this line for you.
  • If you are contributing a new feature, is your work based off the develop branch?
  • If you are contributing a bugfix, is your work based off the fixes branch?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run make test with your changes locally?

Hello

  • Add the correct connack codes (add fixes the usage of it)
  • Add a function for disconnect

@Its-Just-Nans
feat: update mosquitto strings
b4fb803 
Signed-off-by: n4n5 <its.just.n4n5@gmail.com>
@Its-Just-Nans
typo
fd7570a 
Signed-off-by: n4n5 <its.just.n4n5@gmail.com>
@Its-Just-Nans
fix tests
7ad80d5 
Signed-off-by: n4n5 <its.just.n4n5@gmail.com>
@Its-Just-Nans
fix: better text error message
419974f 
Signed-off-by: n4n5 <its.just.n4n5@gmail.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 16, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 92.03540% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
apps/mosquitto_ctrl/client.c 0.00% 2 Missing ⚠️
client/pub_client.c 0.00% 2 Missing ⚠️
client/sub_client.c 0.00% 2 Missing ⚠️
lib/cpp/mosquittopp.cpp 0.00% 2 Missing ⚠️
client/rr_client.c 0.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
lib/srv_mosq.c 0.00% <ø> (ø)
libcommon/strings_common.c 100.00% <100.00%> (ø)
client/rr_client.c 74.19% <0.00%> (ø)
apps/mosquitto_ctrl/client.c 76.56% <0.00%> (+2.32%) ⬆️
client/pub_client.c 81.42% <0.00%> (+0.46%) ⬆️
client/sub_client.c 78.62% <0.00%> (+0.59%) ⬆️
lib/cpp/mosquittopp.cpp 76.34% <0.00%> (-0.64%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ralight ralight force-pushed the develop branch 4 times, most recently from b612b85 to 93557bf Compare August 22, 2025 22:17
@ralight
Copy link
Copy Markdown
Contributor

ralight commented Aug 24, 2025

I don't think I agree with this change. Could you explain what problem you believe it to be solving?

@Its-Just-Nans
Copy link
Copy Markdown
Contributor Author

Its-Just-Nans commented Aug 24, 2025 •
edited
Loading

I don't think I agree with this change. Could you explain what problem you believe it to be solving?

mosquitto_connack_string() is incorrect for the MQTT v5 as it currently handle only code from 0 to 5

mosquitto/libcommon/strings_common.c

Lines 148 to 166 in 7786833

const char *mosquitto_connack_string(int connack_code)
{
switch(connack_code){
case 0:
return "Connection Accepted";
case 1:
return "Connection Refused: unacceptable protocol version";
case 2:
return "Connection Refused: identifier rejected";
case 3:
return "Connection Refused: broker unavailable";
case 4:
return "Connection Refused: bad user name or password";
case 5:
return "Connection Refused: not authorised";
default:
return "Connection Refused: unknown reason";
}
}

Currentlyt, to get the string of a connack code, we should do

  • mosquitto_connack_string() for MQTTv3
  • mosquitto_reason_string() for MQTTv5

And it's also not documented if I'm correct - only as comment here

mosquitto/examples/subscribe/basic-1.c

Lines 17 to 20 in 7786833

/* Print out the connection result. mosquitto_connack_string() produces an
* appropriate string for MQTT v3.x clients, the equivalent for MQTT v5.0
* clients is mosquitto_reason_string().
*/

This fix, update the mosquitto_connack_string for both

Also I've added strings for disconnect code translation using mosquitto_disconnect_string() for the 3.14.2.1 Disconnect Reason Code on https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901208
also adding test, and fixing examples

@ralight
Copy link
Copy Markdown
Contributor

ralight commented Aug 24, 2025

Ah, thank you I understand. I believe the answer is to better document it then. There is no such thing as a connack code/string in MQTT v5.0, and no such thing as a reason code/string in MQTT v3.1.1, so mixing the two doesn't make sense to me.

@Its-Just-Nans
Copy link
Copy Markdown
Contributor Author

Its-Just-Nans commented Aug 24, 2025 •
edited
Loading

Hum that's strange because on MQTTv5 (https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html) I see

  • 2.4 Reason Code
  • 3.2.2.2 Connect Reason Code

On MQTTv3 I see

  • 3.2.2.3 Connect Return code

On MQTTv5, the codes from 2.4 have a general name e.g

Paragraph code
2.4 Reason Code 155 - 0x9B - QoS not supported
3.2.2.2 Connect Reason Code 155 - 0x9B - QoS not supported - The Server does not support the QoS set in Will QoS.
3.14.2.1 Disconnect Reason Code 155 - 0x9B - QoS not supported - The Client specified a QoS greater than the QoS specified in a Maximum QoS in the CONNACK.

What do you think about the mosquitto_disconnect_string()?

What do you think about the mosquitto_connack_string() handling also MQTTv5 code?
For example if(code > 5) return mosquitto_reason_string(code)

I can also edit the documentation if needed

@ralight ralight force-pushed the develop branch 2 times, most recently from 6b404a7 to 2c4967a Compare October 10, 2025 22:17
@ralight ralight force-pushed the develop branch 12 times, most recently from 74cc684 to fd0fa72 Compare December 10, 2025 00:49
@ralight ralight force-pushed the develop branch 2 times, most recently from 6101d88 to 7de6cb4 Compare January 23, 2026 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Its-Just-Nans @ralight